-
Notifications
You must be signed in to change notification settings - Fork 33
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[V5] Init Content API RFC #53
base: main
Are you sure you want to change the base?
Conversation
3382ff8
to
16a3ea3
Compare
rfcs/xxxx-v5-content-api.md
Outdated
|
||
### Endpoints | ||
|
||
Based on the [Database changes planned](https://github.com/strapi/rfcs/pull/52),we will now mainly work with the `documentId` within the API. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am curious about how users can make modifications to a particular entry using the API, given that it accepts documentId as input. For example, let's say I want to delete my entry with entryId : how would it be done with the new API ? (Similar questions can be raised for the other endpoints.)
One potential approach to address this issue (if it's considered as one) could be to introduce a new endpoint. Either /api/:contentType/document/:documentId to keep the old API working as is, or add /api/:contentType/entry/:entryId to at least have an endpoint similar to the old behavior. (edit: another idea is /api/:contentType/:documentId/:entryId though it might be harder to use for no real value)
From my current understanding, multiple entries can refer to the same documentId, and a "document" instance doesn't truly exist by itself (e.g., there isn't a structure like { documentId: '123', entries: [...] }).
If I misunderstood any aspect (I don't have deep expertise in the specifics of your codebase), feel free to ignore my message, though your clarifications will be appreciated. :)
rfcs/xxxx-v5-content-api.md
Outdated
"documentId": "clkgylw7d000108lc4rw1bb6s", | ||
"entryId": 1, | ||
"name": "Category A", | ||
"meta": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This clashed with a component or field having the name "meta", is the later then shadowed?
rfcs/xxxx-v5-content-api.md
Outdated
"name": "Category A", | ||
"meta": { | ||
"locale": "en", | ||
"createdAt": "2023-01-01T00:00:00.000Z", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think publication_state should be added to meta as well. (or all pivot elements)
rfcs/xxxx-v5-content-api.md
Outdated
# Unresolved questions | ||
|
||
- Should we use `id` or `documentId` in the new response format. | ||
- Should we use `entryId` or `entityId` to represent the underlying Ids to use for low level relations |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The API should not rename fields, so if you want to use either of it, the database should be renamed from id to e?id as well.
|
||
Pros: | ||
|
||
- No conflicts between user & system attributes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How can there be conflicts anyway? All fields are added flat to the database table, so noone can name a field created_at.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reference is to conflicts from existing projects with a field like "publishedBy" or "release" that do not exist as internal fields today but if upgraded to v5 would be a breaking change to those users and result in a conflict. Likewise if we decide to add any other system/internal fields during the v5 lifecycle.
rfcs/xxxx-v5-content-api.md
Outdated
| -------- | ----------------------------------------------- | ----------------------------------------------------------------------- | | ||
| `GET` | `/api/:contentType` | Find a list of documents | | ||
| `POST` | `/api/:contentType` | Create a document | | ||
| `GET` | `/api/:contentType/:documentId` | Find a document | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think both should be possible:
/api/:contentType/:documentId
and
/api/:contentType/:entityId.
rfcs/xxxx-v5-content-api.md
Outdated
|
||
| Method | Url | Desc | | ||
| -------- | ----------------------------------------------- | ----------------------------------------------------------------------- | | ||
| `GET` | `/api/:contentType` | Find a list of documents | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This feels like filtering for pivot states will be baked into filters api. I think it would benefit, if there would be clear GET endpoints for like "all french documents" or "all live documents".
I'm not sure where to comment on the RFC, so I'll add my comments here: The simplified API response is a welcome feature 🥰. I do however have my concerns about the usage of 'documentId'. It isn't clear to me this response will also be used for the GraphQL response, but last I checked, using something like Apollo with caching, requires an 'id' field. Maybe this is configurable, but next to that, I don't really see the benefits of using something other than 'id'. Rest looks super solid! |
As mentioned above I would also advocate for keeping id as id instead of renaming it to documentId. id is a term everyone is already familiar with and understand what it is for (uid of a record). I would say we keep ID for the uid of every record and the some other field (entryId sounds good to me) for indicating the id for the base record. |
Thank you all for your comments 🚀 We have an updated version coming next week that thanks to your notes 🔥 |
8f5f31e
to
8badeb6
Compare
8badeb6
to
6fabcb9
Compare
Hello there 👋 There RFC was just updated to clarify certain differences between internal & meta attributes & show the different options that we have. If you can share what you think of each of them for your own DX that would be great 💯 |
The updated version looks a lot better now! Given the option return formats I prefer option 2, but with it being meta not internal Reasoning
Looking forward! |
This gives me an other question. no matter what option you do. how would you for come name conflicts on the DB layer. Also lets say we go for option 3. would that mean none can start a string with a _ or $ since if you where to allow for that that would mean you can still have conflicts. |
Depending on the option we go with for the DX here we will of course adapt the DB layer to use prefixes when necessary. It would of course mean we forbidden those prefixes to be used (which is already the case) |
Other question I see no where in the RFC how you would reference a specific entry in the document. |
the document is a virtual thing fetching a document will always return the content of a single entry. (Based on filters provided or default filters) if you do (just an example) |
I think you should also be able to request by id since how would you get a record where a new one has been posted over so it no longer is first in any combination of keys also there needs to be an endpoint that lists all enteries in a document. |
Hello everyone. Thank you all for your comments 💯 We will go ahead and update this RFC to reflect the option we are going to move forward with and merge it in the upcoming days. After carefully considering the different Options, Option 1 is the most viable one for v5 and is the one we will be moving forward with. Here are the main driving deciders
|
I have been using Strapi for a short while now, and the nesting of attributes has been somewhat a hassle. Luckily, with ChatGPT nowadays the generation of all data classes (I consume in Kotlin) made it easier. For a smooth migration, I think the best option is to supply a request parameter that allows for choosing the format. That way, all consumers of content can prepare for a new version and then a v5.3.x for example can migrate the version to be the new one by default. What I am missing, how would the localizations be added under |
Introduce the Content API changes for v5
You can read it here